Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIX] Compatibility for OpenERP versions prior to 6.1 #21

Merged
merged 1 commit into from
Oct 2, 2015

Conversation

pedrobaeza
Copy link
Member

Closes OCA/OpenUpgrade#373

@sebalix I'm afraid you should check if all used methods are totally compatible with these versions, as the used version for all of them has been 8.0, but with this patch you will be able to load OpenUpgrade 6.0 branch (and I have also checked 5.0).

cc @StefanRijnhart

rel, id1, id2 = openerp.osv.fields.many2many._sql_names(
model._columns[field], model)
if isinstance(model._columns[field], (many2many, Many2many)):
rel, id1, id2 = many2many._sql_names(model._columns[field], model)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you'll want to take self._rel, self._id1, self._id2 for version < 6.1, because the _sql_names method was introduced then.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for my confusing use of self in the initial comment. You take the fields from model, but should that not be the column itself?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad also for not thinking twice (it's sunday 😜). Fixed.

@StefanRijnhart
Copy link
Member

Thanks for dealing with this one!

@sebalix
Copy link

sebalix commented Sep 27, 2015

@pedrobaeza I will test that, thank you

@StefanRijnhart
Copy link
Member

LGTM ;-) 👍

@sebalix
Copy link

sebalix commented Sep 28, 2015

Hi @pedrobaeza

I just tested your branch, and I have a new error now:

ERROR:OpenUpgrade:base: error in migration script /home/openerp/oerp/server/bin/addons/base/migrations/6.0.1.3/pre-migration.py: 'module' object has no attribute 'version_info'
[2015-09-28 16:44:25,664][database] ERROR:OpenUpgrade:'module' object has no attribute 'version_info'
Traceback (most recent call last):
  File "/usr/local/lib/python2.7/dist-packages/openupgradelib-1.1.0-py2.7.egg/openupgradelib/openupgrade.py", line 926, in wrapped_function
    func(cr, version)
  File "base/migrations/6.0.1.3/pre-migration.py", line 108, in migrate
    cr, module_namespec
  File "/usr/local/lib/python2.7/dist-packages/openupgradelib-1.1.0-py2.7.egg/openupgradelib/openupgrade.py", line 608, in update_module_names
    if release.version_info[0] > 7:
AttributeError: 'module' object has no attribute 'version_info'
[2015-09-28 16:44:25,665][database] ERROR:migration:module base: Each pre-migration file must have a "migrate(cr, installed_version)" function

The release module does not contain the version_info variable in the 6.0 branch, just this:

version = '6.0.4'
major_version = '6.0'

And the release.version_info pattern is used at differents places in openupgradelib:

./tests/test_openupgradelib.py:19:openerp_mock.release.version_info = (8, 0, 0, 'final', 0)
./openupgradelib/openupgrade.py:39:    if release.version_info[0] >= 7:
./openupgradelib/openupgrade.py:41:    if release.version_info[0] >= 8:
./openupgradelib/openupgrade.py:608:        if release.version_info[0] > 7:
./openupgradelib/openupgrade.py:641:        map(str, release.version_info[0:2]))+'_'+original_name
./openupgradelib/openupgrade.py:1065:    if release.version_info[0] < 7:

Maybe we should encapsulate the detection of the version in a new function?

@pedrobaeza
Copy link
Member Author

Yeah, I missed that one. I already did it in this one: https://github.com/OCA/openupgradelib/pull/21/files#diff-3d7675eb1386de028dab1f879bdc4681R33, but there are some other places where vendor_info is used.

The problem I see encapsulating the version detection is that each new Odoo version will need to add some logic to this encapsulation method, but if you see this correct, we can make it via CONSTANTS with growing integers that allows to make comparisons with <, =, >, <=, and we can fallback to a higher value in case none of the current versions are present. What do you think?

VERSION_42 = 1
VERSION_50 = 2
VERSION_60 = 3
VERSION_61 = 4
VERSION_70 = 5
VERSION_80 = 6
VERSION_90 = 7
VERSION_UNKNOWN = 99
...
if not hasattr(release, 'version_info'):
    if x:
        version = VERSION_60
   else:
...

@sebalix
Copy link

sebalix commented Sep 28, 2015

The problem with this system is the get_legacy_name() function which needs the real version number to generate the field's name, no?

So I was thinking about a function like this:

def get_version():
    if not hasattr(release, 'version_info'):
        return tuple(map(int, release.version.split('.')))    # add ('final, 0) if it really matters
    else:
        return release.version_info()

And replacing release.version_info everywhere by a call to get_version(). What do you think?

@pedrobaeza
Copy link
Member Author

Good idea! I think this can be good enough to overpass this problem, although I think defining a method prior imports is not allowed by flake8.

@sebalix
Copy link

sebalix commented Sep 28, 2015

If flake8 complains about this, we could put the function in another (new) module?

@pedrobaeza
Copy link
Member Author

I'll check tonight and tell you

@pedrobaeza
Copy link
Member Author

OK, let's hope this one is the last needed modification! I have saved in version_info module variable the tuple (previously obtained according version), and thus the conditional can be the same in all places.

cc @StefanRijnhart @sebalix

@sebalix
Copy link

sebalix commented Sep 29, 2015

Hi @pedrobaeza
Now I can launch the 6.0 server, great.

I have another error which occurs later but I don't know if its related to openupgradelib or the base module migration script of the 6.0 branch or the database itself. All I know is that the migration 5.0 -> 6.0 was successful before the integration of the openupgradelib, so maybe that some errors was not raised before or something like that. If it can help, the traceback:

DEBUG:OpenUpgrade:model res.users, field company_ids: setting default value of resources [4, 3, 2, 1] to [1]
[2015-09-29 12:30:09,176][database] ERROR:db.cursor:Programming error: column "company_ids" of relation "res_users" does not exist
LINE 1: UPDATE res_users SET company_ids = ARRAY[1] WHERE id IN (1, ...
                             ^
, in query UPDATE res_users SET company_ids = %s WHERE id IN %s
[2015-09-29 12:30:09,176][database] ERROR:OpenUpgrade:base: error in migration script /home/openerp/oerp/server/bin/addons/base/migrations/6.0.1.3/post-migration.py: column "company_ids" of relation "res_users" does not exist
LINE 1: UPDATE res_users SET company_ids = ARRAY[1] WHERE id IN (1, ...
                             ^

[2015-09-29 12:30:09,176][database] ERROR:OpenUpgrade:column "company_ids" of relation "res_users" does not exist
LINE 1: UPDATE res_users SET company_ids = ARRAY[1] WHERE id IN (1, ...
                             ^
Traceback (most recent call last):
  File "/usr/local/lib/python2.7/dist-packages/openupgradelib-1.1.0-py2.7.egg/openupgradelib/openupgrade.py", line 930, in wrapped_function
    func(cr, version)
  File "base/migrations/6.0.1.3/post-migration.py", line 305, in migrate
    openupgrade.set_defaults(cr, pool, defaults)
  File "/usr/local/lib/python2.7/dist-packages/openupgradelib-1.1.0-py2.7.egg/openupgradelib/openupgrade.py", line 548, in set_defaults
    obj._defaults[field](obj, cr, user_id, None))
  File "/usr/local/lib/python2.7/dist-packages/openupgradelib-1.1.0-py2.7.egg/openupgradelib/openupgrade.py", line 514, in write_value
    cr.execute(query, (params[0], sub_ids))
  File "/home/openerp/oerp/server/bin/sql_db.py", line 78, in wrapper
    return f(self, *args, **kwargs)
  File "/home/openerp/oerp/server/bin/sql_db.py", line 131, in execute
    res = self._obj.execute(query, params)
ProgrammingError: column "company_ids" of relation "res_users" does not exist
LINE 1: UPDATE res_users SET company_ids = ARRAY[1] WHERE id IN (1, ...
                             ^

Traceback (most recent call last):
  File "/home/openerp/oerp/server/bin/openerp-server.py", line 121, in <module>
    db,pool = pooler.get_db_and_pool(dbname, update_module=tools.config['init'] or tools.config['update'], pooljobs=False)
  File "/home/openerp/oerp/server/bin/pooler.py", line 39, in get_db_and_pool
    addons.load_modules(db, force_demo, status, update_module)
  File "/home/openerp/oerp/server/bin/addons/__init__.py", line 975, in load_modules
    processed_modules.extend(load_module_graph(cr, graph, status, perform_checks=(not update_module), registry=registry, report=report, skip_modules=skip_modules))
  File "/home/openerp/oerp/server/bin/addons/__init__.py", line 903, in load_module_graph
    migrations.migrate_module(package, 'post')
  File "/home/openerp/oerp/server/bin/addons/__init__.py", line 598, in migrate_module
    mod.migrate(self.cr, pkg.installed_version)
  File "/usr/local/lib/python2.7/dist-packages/openupgradelib-1.1.0-py2.7.egg/openupgradelib/openupgrade.py", line 930, in wrapped_function
    func(cr, version)
  File "base/migrations/6.0.1.3/post-migration.py", line 305, in migrate
  File "/usr/local/lib/python2.7/dist-packages/openupgradelib-1.1.0-py2.7.egg/openupgradelib/openupgrade.py", line 548, in set_defaults
    obj._defaults[field](obj, cr, user_id, None))
  File "/usr/local/lib/python2.7/dist-packages/openupgradelib-1.1.0-py2.7.egg/openupgradelib/openupgrade.py", line 514, in write_value
    cr.execute(query, (params[0], sub_ids))
  File "/home/openerp/oerp/server/bin/sql_db.py", line 78, in wrapper
    return f(self, *args, **kwargs)
  File "/home/openerp/oerp/server/bin/sql_db.py", line 131, in execute
    res = self._obj.execute(query, params)
psycopg2.ProgrammingError: column "company_ids" of relation "res_users" does not exist
LINE 1: UPDATE res_users SET company_ids = ARRAY[1] WHERE id IN (1, ...

@StefanRijnhart
Copy link
Member

That is a regression of changing the default method of set_defaults to SQL, which does not handle x2many fields properly. It could be improved on that point, or you could call set_defaults for these fields separately with an additional use_orm=True argument.

@pedrobaeza
Copy link
Member Author

As said by @StefanRijnhart, it's not something of this PR. We can go on then to fix this and then make another PR for fixing the other problem.

@sebalix
Copy link

sebalix commented Oct 1, 2015

👍

@pedrobaeza
Copy link
Member Author

@StefanRijnhart, can you please merge?

@StefanRijnhart
Copy link
Member

With pleasure!

StefanRijnhart added a commit that referenced this pull request Oct 2, 2015
[FIX] Compatibility for OpenERP versions prior to 6.1
@StefanRijnhart StefanRijnhart merged commit 9329fd8 into OCA:develop Oct 2, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants